Skip to content

Fix kg_emb broken import: SampleBaseDataset -> SampleDataset, add pan… - #1192

Open
userjuma wants to merge 1 commit into
sunlabuiuc:masterfrom
userjuma:fix-kg-emb-broken-import
Open

Fix kg_emb broken import: SampleBaseDataset -> SampleDataset, add pan…#1192
userjuma wants to merge 1 commit into
sunlabuiuc:masterfrom
userjuma:fix-kg-emb-broken-import

Conversation

@userjuma

Copy link
Copy Markdown

Closes #952

Fixes the broken import in kg_emb caused by SampleBaseDataset being renamed to SampleDataset in PyHealth 2.0. Updated across all 7 affected files (2 in kg_emb/datasets, 5 in kg_emb/models).

Also adds pandarallel (>=1.6.5) to pyproject.toml. It is used in kg_emb/datasets/base_kg_dataset.py but was previously missing from declared dependencies, which caused a ModuleNotFoundError immediately after the dataset rename was resolved.

Added tests/core/test_kg_emb.py covering:
1.Clean import of the kg_emb module
2. Importability of all five kg_emb model classes
3.A regression check ensuring TransE's dataset parameter is explicitly typed as SampleDataset

Manual verification:
Reproduced the original crash via python -c "import pyhealth.medcode.pretrained_embeddings" and confirmed it now resolves cleanly end-to-end.

@AxelNoun

Copy link
Copy Markdown
Contributor

Thanks for this. The ImportError diagnosis is right, and the failure chain in
your description, SampleBaseDataset first and then pandarallel, matches what
I reproduced exactly.

I think one step is missing though. SampleDataset in 2.0 isn't the successor
to SampleBaseDataset: it's a litdata.StreamingDataset that expects a
directory containing schema.pkl. SampleKGDataset.__init__ still passes a
list, so:

ValueError: dir_path must be either a string, Path, or Dir, got: <class 'list'>

The module imports, but set_task() can't return anything usable. The three
tests here don't catch it because none of them instantiates the class.

Two smaller notes:

  • pandarallel: initialize() is indeed called in umls.py, but the import in
    base_kg_dataset.py, the one cited in the description, is dead, and there is
    no parallel_apply anywhere in kg_emb. It is also not declared in
    pyproject.toml on master, which is what produced the ModuleNotFoundError
    in the first place.
  • The test comparing TransE.__init__'s annotation to SampleDataset locks in
    an incorrect hint: KGEBaseModel reads dataset.entity_num, which only
    exists on SampleKGDataset.

Worth saying that #952 asked for the rename across eight files and your PR does
exactly that, so this is really about how the issue was framed.

I've opened #1202 going after the root cause, and credited you in the
description. Would be glad to have your review on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: kg_emb broken import in PyHealth 2.0

2 participants